These three tests are unnecessarily strict.

Relaxing them should help avoid spurious test failures in the future.

If someone changes the return type of gcd(Int[]) or gcd(Rational{Int}[]), I want a test to fail. Similarly, if someone changes the return type or value of lcm(Int[]), I want a test to fail. Those would be breaking changes that should not be made accidentally.

I'm adding them because I noticed that #56113 would break all three of these new tests but doesn't break any existing tests; I want to make sure that behavior change is only made if folks are aware that we are changing existing behavior.

Changing the return type isn't breaking, as long as the value is correct and the type subtypes the correct abstract type.

Making a mathematical operation on Ints that used to return Ints instead return a different type could introduce errors or correctness bugs based on overflow behavior changes or failed conversions and could also introduce significant performance issues through the introduction of type instability in user code.

Changing 1+1 to return an Int8 is breaking even through Int8 has all the same abstract supertypes as Int. A similar change to lcm or gcd would be much less impactful because + is so much more commonly used but would still be breaking.

IMO, this is wrong. Rational numbers are a field, and as such to not have an LCM

xref #56166

I think it is perfectly sensible to define lcm on rationals, provided we choose a definition for "multiple" that is based on integers. A definition that is mathematically sensible and in line with current behavior (except for #55379) is

For a signed ring R, m in R is an lcm of a_1, a_2, ... a_n in R iff m is positive and is a multiple of a_1, a_2, ... and a_n and for all other m' in R that are multiples of a_1, a_2, ... and a_n, m' is a multiple of m.

And we define "multiple" as

a is a multiple of b iff there exists an integer n such that b * n = a where * refers to multiplication in the ring of integers, not in the ring of integers mod 2^64.

While there is certainly a case to be made that lcm should throw on rationals, now is not the time to make that case. That time was #33910. lcm has supported rationals under the definition above since 1.4.

I am not aware of any definitions of lcm over the integers or the rationals that would result in returning different answers; though some definitions should throw errors:

Using "m <= m'" instead of "m' is a multiple of m" produces the same results on rationals and integers.

Defining "multiple" based on the existence of a ring element that takes b to a instead of based on an integer that takes b to a is equivalent for the ring of integers. For the rationals, every positive rational is the lcm of every set of rationals which doesn't seem like a particularly useful definition. Folks sometimes (e.g. #27039 (comment), #56166 (comment)) say that lcm(a,b) == 1 for a field which, if we are defining multiple as "all field elements are multiples of all other field elements", is not wrong but we could just as correctly say that lcm(a,b) == 17 for all field elements a and b under that definition which means the right answer for a programming language is an error, not the number 1 nor the number 17.

I took a different route to obtain the same answer here #55379 (comment)

with the FTA based definition, lcm(1//2, 1//2) == 1//2 . in my opinion it is perfectly clean & consistent this way.

Ah yes, the FTA definition is nice. AFAICT these two definitions agree in all cases.

For the empty case over the rationals the definition I listed indicates there is no LCM. Using the FTA definition extended to rationals requires computing max over an empty set of integers which also does not exist.

as counterpoint to my interpretation, it would lead to gcd([]) = 1 as well, which is normatively worse than gcd([]) = 0

to be honest though, I think all these choices are basically just convention and I don't think it matters that much that there is some underlying purity determining the convention as long as what we end up with makes sense.

I think the current state of this PR makes good choices for each convention, and afaict it looks like the court of public opinion --- which seems to arise from the recursive definitions --- agrees https://math.stackexchange.com/questions/1755266/gcd-of-an-empty-set https://www.reddit.com/r/learnmath/comments/v9vmfm/whats_the_empty_lcm/

@adienes, please take a look at #56166

all these choices are basically just convention and I don't think it matters that much that there is some underlying purity determining the convention as long as what we end up with makes sense

The only way to end up with something that makes sense is to respect the math ("underlying purity").

respectfully, I don't think "the math" makes any particularly consistent demands about what to do in these edge cases. and any choice made is essentially just convention (which can and does vary among authors)

As described in the linked issue, there indeed are multiple possible choices here. That doesn't imply that anything goes, though.

I think a more charitable interpretation of my comments (rather than "anything" goes) is "a lot of things could go, and I think the choices made here are good ones"

but anyway I guess let's just leave to to triage. I don't typically attend triage but if the discussion comes up, I would upvote the implementation in this PR.